Minor changes required for nnet3 sequence training by vimalmanohar · Pull Request #560 · kaldi-asr/kaldi

vimalmanohar · 2016-03-06T02:39:45Z

No description provided.

danpovey · 2016-03-06T02:45:26Z

Thanks- will try to review soon. @vijayaditya, if you have time please take a look.

vijayaditya · 2016-03-06T03:19:54Z

OK, I will work on this after the ivector PR from Yiming.

--VIjay

On Sat, Mar 5, 2016 at 9:45 PM, Daniel Povey notifications@github.com
wrote:

Thanks- will try to review soon. @vijayaditya
https://github.com/vijayaditya, if you have time please take a look.

—
Reply to this email directly or view it on GitHub
#560 (comment).

vimalmanohar · 2016-03-06T04:24:34Z

Note: This is not the full set of changes. There is a lot of codes and scripts, which is beyond what the github diff allows to view. So I am committing them in small chunks that would be easier to look at. This is first set of commits.

vimalmanohar · 2016-03-06T04:25:25Z

I can push the next set of commits once this is merged.

danpovey · 2016-03-06T23:12:11Z

src/lat/lattice-functions.h

+// negated costs.  Requires that lat be topologically sorted.  This code
+// will work for either CompactLattice or Latice.
+template<typename LatticeType>
+double ComputeLatticeAlphasAndBetas(const LatticeType &lat,


vimal, it's not good for compilation speed to define this in the header.
Just declare it in the header, and in the .cc file, define it and instantiate it for float and double.

danpovey · 2016-03-07T00:04:26Z

OK, done. Please merge or rebase.

vimalmanohar · 2016-03-07T00:22:34Z

I have changed the functions to use SetUnderlyingLearningRate

danpovey · 2016-03-07T00:33:11Z

src/nnet3/nnet-utils.cc

+      if (uc == NULL)
+        KALDI_ERR << "Updatable component does not inherit from class "
+            "UpdatableComponent; change this code.";
+      uc->SetUnderlyingLearningRate(uc->LearningRate() * learning_rate_scale);


you actually want to use SetActualLearningRate() here.

vimalmanohar · 2016-03-07T01:15:23Z

I made the change.

danpovey · 2016-03-07T01:19:46Z

src/lat/lattice-functions.h

                                vector<double> *beta);

+
+static inline double LogAddOrMax(bool viterbi, double a, double b) {


do you still need this function to be in the header?

Its a small function that can be inlined. If it is moved to .cc file, it
cannot be inlined.

On Sun, Mar 6, 2016 at 8:19 PM Daniel Povey notifications@github.com
wrote:

In src/lat/lattice-functions.h
#560 (comment):

@@ -74,6 +74,26 @@ bool ComputeCompactLatticeAlphas(const CompactLattice &lat,
bool ComputeCompactLatticeBetas(const CompactLattice &lat,
vector *beta);

+static inline double LogAddOrMax(bool viterbi, double a, double b) {

do you still need this function to be in the header?

—
Reply to this email directly or view it on GitHub
https://github.com/kaldi-asr/kaldi/pull/560/files#r55153738.

Vimal Manohar
PhD Student
Electrical & Computer Engineering
Johns Hopkins University

so you are going to call it from some other code you have written?

also if you are going to leave it in the header you should take away the 'static'.

vimalmanohar · 2016-03-07T03:00:35Z

It seems there was only function that used it, which is not back in the .cc file. So I moved it to the .cc file.

danpovey · 2016-03-07T03:34:12Z

src/lat/lattice-functions.cc

  return true;
 }

+static inline double LogAddOrMax(bool viterbi, double a, double b) {


ok, you've now just moved this code around. could you revert this file?

danpovey · 2016-03-07T03:35:04Z

and please squash your commits.

vimalmanohar · 2016-03-07T03:44:09Z

Done

danpovey · 2016-03-07T03:47:23Z

src/nnet3/nnet-example-utils.h

+                             bool binary,
+                             Vector<BaseFloat> *vec);
+
+void RoundUpNumFrames(int32 frame_subsampling_factor,


you should document this declaration.

danpovey reviewed Mar 6, 2016
View reviewed changes

vimalmanohar force-pushed the sequence_changes branch 2 times, most recently from cb187d9 to 541e4ae Compare March 7, 2016 00:20

danpovey reviewed Mar 7, 2016
View reviewed changes

vimalmanohar force-pushed the sequence_changes branch from 888ddc2 to 5bd5fb4 Compare March 7, 2016 02:58

danpovey reviewed Mar 7, 2016
View reviewed changes

vimalmanohar force-pushed the sequence_changes branch from 5bd5fb4 to 10fbf54 Compare March 7, 2016 03:43

danpovey reviewed Mar 7, 2016
View reviewed changes

vimalmanohar force-pushed the sequence_changes branch from 10fbf54 to cf7a66b Compare March 7, 2016 04:04

		vector<double> *beta);


		static inline double LogAddOrMax(bool viterbi, double a, double b) {

Conversation

vimalmanohar commented Mar 6, 2016

Uh oh!

danpovey commented Mar 6, 2016

Uh oh!

vijayaditya commented Mar 6, 2016

Uh oh!

vimalmanohar commented Mar 6, 2016

Uh oh!

vimalmanohar commented Mar 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danpovey commented Mar 7, 2016

Uh oh!

vimalmanohar commented Mar 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vimalmanohar commented Mar 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vimalmanohar commented Mar 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danpovey commented Mar 7, 2016

Uh oh!

vimalmanohar commented Mar 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants